fix: preserve AI attribution when human edits file after AI checkpoint (fixes #1444)#1667
fix: preserve AI attribution when human edits file after AI checkpoint (fixes #1444)#1667IanMcNelly wants to merge 3 commits into
Conversation
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| let displaced_to_initial_lines: HashMap<String, HashSet<u32>> = initial_attributions | ||
| .files | ||
| .iter() | ||
| .filter_map(|(file, line_attrs)| { | ||
| let committed_for_file = recovery_hunks.get(file)?; | ||
| let committed_set: HashSet<u32> = | ||
| committed_for_file.iter().flat_map(|r| r.expand()).collect(); | ||
| let displaced: HashSet<u32> = line_attrs | ||
| .iter() | ||
| .flat_map(|la| la.start_line..=la.end_line) | ||
| .filter(|l| committed_set.contains(l)) | ||
| .collect(); | ||
| if displaced.is_empty() { | ||
| None | ||
| } else { | ||
| Some((file.clone(), displaced)) | ||
| } | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
🚩 displaced_to_initial_lines assumes carryover and committed coordinates are identical
The displaced_to_initial_lines computation at src/authorship/post_commit.rs:400-418 intersects initial_attributions line numbers (carryover/working-directory coordinates) with recovery_hunks line numbers (committed coordinates). These coordinate spaces differ when there are pure insertions in carryover. However, I traced through the scenarios and found that when pure insertions exist (human appends lines after AI checkpoint), the stale-observed detection at src/authorship/virtual_attribution.rs:2058-2062 fires and sets carryover = committed, eliminating all unstaged hunks and producing empty initial_attributions. For replacements (human modifies a line), there are no pure insertions, so coordinates are identical. The assumption appears safe for the targeted scenarios, but if future changes alter the stale-detection logic, this intersection could silently produce incorrect results.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed safe for the current scenarios.
But, I added a comment documenting the invariant and why it holds:
stale-detection in build_carryover_snapshot fires for all cases where pure insertions would exist (human appends/inserts), replacing the carryover with committed content and emptying initial_attributions for those files. The only case that populates initial_attributions is in-place replacement, which has no coordinate shift. Added a note about what would need to change (coordinate-aware conversion using pure_insertion_hunks) if the stale-detection path is ever altered.
When a human modifies a file after an AI checkpoint fires but before committing — without triggering a known_human checkpoint — the carryover snapshot could be stale (holding the AI version). This caused the overlap filter to incorrectly strip the human-modified line from unstaged_hunks, leaving it attributed to AI. Fix: skip the committed-vs-unstaged overlap filter entirely when a carryover snapshot is present. With a snapshot, Replace-type unstaged lines represent lines where committed content differs from the AI checkpoint — they should flow to INITIAL, not be re-attributed. Also replace the content-based guard in recover_adjacent_edges with a displaced_to_initial_lines set derived from initial_attributions ∩ recovery_hunks. This prevents edge recovery from re-claiming human-modified lines that were explicitly sent to INITIAL, while still allowing edge recovery for adjacent uncheckpointed lines (inserts/appends) that were never in the attribution loop at all. Fixes git-ai-project#1444 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-fill Pure-insertion unstaged lines (present in carryover but absent from the commit) have no committed counterpart. Computing a committed coordinate for them and inserting it into explicitly_resolved_commit_lines fabricates a position that aliases an unrelated committed line, potentially blocking gap-fill attribution for that line. Add a binary_search guard so only Replace-type unstaged lines (which have a real committed counterpart) claim a committed-coordinate slot. Also document the coordinate-space invariant in the displaced_to_initial_lines computation: stale-detection ensures initial_attributions is empty for files with pure insertions, so the direct intersection of carryover and committed coordinates is safe for all currently targeted scenarios.
2 participants
Problem
When a human modifies a file after an AI checkpoint fires but before committing — without a
known_humancheckpoint being triggered — AI attribution for the AI's own lines was lost (shown as 100% untracked).Reported in #1444.
Root cause
The carryover snapshot stays stale (holds the AI checkpoint version). An overlap filter in the attribution loop was then stripping the human-modified position from
unstaged_hunksbecause it also appeared incommitted_hunks— incorrectly crediting that committed line to AI. For the modify case (human replaces an AI-written line), this meant the human's content was attributed to AI.Fix
virtual_attribution.rs: Skip the committed-vs-unstaged overlap filter when a carryover snapshot is present. With a snapshot, Replace-type unstaged lines represent content divergence between the AIcheckpoint and the commit — they should flow to INITIAL, not be silently re-attributed.
attribution_recovery.rs/post_commit.rs: Replace the content-based guard inrecover_adjacent_edgeswith adisplaced_to_initial_linesset (intersection ofinitial_attributionsandrecovery_hunks). This blocks edge recovery from re-claiming lines explicitly sent to INITIAL, while preserving edge recovery for genuinely adjacent uncheckpointed inserts/appends.Tests
Added
tests/integration/human_edit_after_ai_checkpoint.rswith four regression tests covering:All existing tests pass including the
delayed_checkout/switch_merge_trace_replaytests.Fixes #1444